Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: LinkedIn Logo #6932

Merged
merged 5 commits into from
Jul 22, 2024
Merged

fix: LinkedIn Logo #6932

merged 5 commits into from
Jul 22, 2024

Conversation

Jay-Karia
Copy link
Contributor

@Jay-Karia Jay-Karia commented Jul 17, 2024

Description

Validation

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run format to ensure the code follows the style guide.
  • I have run npm run test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

Closes #6928

Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
@Jay-Karia Jay-Karia requested a review from a team as a code owner July 17, 2024 17:01
Copy link

vercel bot commented Jul 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jul 19, 2024 1:09pm

Co-authored-by: Augustin Mauroy <augustin.mauroy@outlook.fr>
Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
Copy link

github-actions bot commented Jul 18, 2024

Unit Test Coverage Report

Unit Test Report

Tests Skipped Failures Errors Time
131 0 💤 0 ❌ 0 🔥 5.261s ⏱️

@canerakdas
Copy link
Member

IMO, the error we got in Build should be solved with #6933. Could you rebase please @Jay-Karia 🙇

Copy link
Member

@canerakdas canerakdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for your contribution :shipit:

@AugustinMauroy
Copy link
Contributor

I didn't understand all of LinkedIn's trademark policy, but shouldn't the logo always be in blue?
#3966AD (it's the blue of the logo)

@Jay-Karia
Copy link
Contributor Author

Jay-Karia commented Jul 20, 2024

Should we merge this ?? 🙃

@AugustinMauroy
Copy link
Contributor

Why not but we need to answer to my questions

@ovflowd
Copy link
Member

ovflowd commented Jul 20, 2024

erstand all of LinkedIn's trademark policy, but shouldn't the logo always be in blue?
#3966AD (it's the blue of the logo)

Right the logo should be blue, not black.

@canerakdas
Copy link
Member

canerakdas commented Jul 20, 2024

Can't we use the monochrome version(Black, Reversed white) of the LinkedIn logo? When I look at the contrast ratio (#3966AD), it does not seem to provide a sufficient contrast ratio for the our dark theme;

image

I think it would be better to use monochrome versions as we can provide a better contrast ratio 👀 What do you think @ovflowd @AugustinMauroy

@ovflowd
Copy link
Member

ovflowd commented Jul 20, 2024

Can't we use the monochrome version(Black, Reversed white) of the LinkedIn logo? When I look at the contrast ratio (#3966AD), it does not seem to provide a sufficient contrast ratio for the our dark theme;

image I think it would be better to use monochrome versions as we can provide a better contrast ratio 👀 What do you think @ovflowd @AugustinMauroy

Are those official/allowed LinkedIn contrast schemas? We can't change the logo... If yes, then let's proceed with it.

@Jay-Karia
Copy link
Contributor Author

We never approve the following types of requests because they violate our Terms of Service. So please don’t ask permission to:
• Modify our trademarks or combine them with any other symbols, words, images, designs, or incorporate them into a slogan,

from the official LinkedIn branding policy
https://brand.linkedin.com/policies

@canerakdas
Copy link
Member

Can't we use the monochrome version(Black, Reversed white) of the LinkedIn logo? When I look at the contrast ratio (#3966AD), it does not seem to provide a sufficient contrast ratio for the our dark theme;
image
I think it would be better to use monochrome versions as we can provide a better contrast ratio 👀 What do you think @ovflowd @AugustinMauroy

Are those official/allowed LinkedIn contrast schemas? We can't change the logo... If yes, then let's proceed with it.

When I look at the document, there are black and reversed white versions;

image

And the branding assets created for China also have black and reversed white versions. When I look at the available documents, I see only the white version. I think the safest way would be to go with the blue one and update it when we find a better version in the future 😞

@Jay-Karia
Copy link
Contributor Author

Can we go for Trademark request ? (if possible)

@bmuenzenmeyer
Copy link
Collaborator

a black / white logo is common practice and explicitly called out - we are overthinking it if we think we must use only blue

While our default logo is blue, use the black or white version on layouts that are black and white only. You can also use the white version on dark backgrounds so it's easy to see.

from https://brand.linkedin.com/downloads

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AS-IS - this have my approval

@bmuenzenmeyer
Copy link
Collaborator

merging this - it's adherent to my reading of the policy - no longer in a circle, and the color scheme is correct for light/dark use cases

@bmuenzenmeyer bmuenzenmeyer added this pull request to the merge queue Jul 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 22, 2024
@bmuenzenmeyer bmuenzenmeyer added this pull request to the merge queue Jul 22, 2024
Merged via the queue into nodejs:main with commit 00e19c0 Jul 22, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

icon: need update LinkedIn
5 participants